Skip to content

Add CorruptedTsFileException for TsFile corruption errors with file path#18304

Open
shuwenwei wants to merge 6 commits into
masterfrom
tsfile-corruption-error-message
Open

Add CorruptedTsFileException for TsFile corruption errors with file path#18304
shuwenwei wants to merge 6 commits into
masterfrom
tsfile-corruption-error-message

Conversation

@shuwenwei

Copy link
Copy Markdown
Member

Description

When a TsFile is corrupted during query execution, the original IOException from the TsFile reader is wrapped as a generic RuntimeException without the corrupted file path, making it impossible for users to locate the problem file.

This PR introduces CorruptedTsFileException to carry the corrupted TsFile path and the stage at which corruption was detected.

Key design decisions:

  • RuntimeException: so operator-layer catch (IOException) don't intercept it
  • super(message) + addSuppressed(cause): prevents getRootCause() from penetrating to the original IOException, allowing AbstractDriverThread to match it by instanceof
  • Staged error reporting: 5 stages: READ_TIMESERIES_METADATA, READ_CHUNK_DATA, LOAD_PAGE_READER, DECODE_PAGE_DATA, READ_METADATA_INDEX_NODE
  • Differentiated messages: normal queries mention corruption without file path (tell user to check logs); read_tsfile queries include the full path
  • File references (not String paths) for memory efficiency

Files changed:

  • New: CorruptedTsFileException.java — exception with Stage enum and File reference
  • ErrorHandlingUtils.java — handle CorruptedTsFileException with TSFILE_PROCESSOR_ERROR
  • AbstractDriverThread.java — match CorruptedTsFileException as abort cause
  • FileLoaderUtils.java — wrap IOException at READ_TIMESERIES_METADATA, READ_CHUNK_DATA, LOAD_PAGE_READER stages
  • SeriesScanUtil.java — wrap IOException at DECODE_PAGE_DATA stage
  • DiskChunkLoader.java / DiskAlignedChunkLoader.java — expose getTsFile() for File reference
  • ExternalTsFileQueryResource.java — wrap at READ_METADATA_INDEX_NODE stage
  • i18n: DataNodeQueryMessages.java (en + zh) — EXCEPTION_ constants for all 5 stages
  • IT: IoTDBQueryWithCorruptedTsFileIT.java — 3 test cases (metadata index + page data corruption for read_tsfile, page data for normal query)

…ath during query execution

- Introduce CorruptedTsFileException extending RuntimeException with Stage enum
  (READ_TIMESERIES_METADATA, READ_CHUNK_DATA, LOAD_PAGE_READER,
   DECODE_PAGE_DATA, READ_METADATA_INDEX_NODE)
- Uses super(message) + addSuppressed(cause) to prevent getRootCause penetration
- Carries File reference and Stage for precise error reporting
- Catch points in FileLoaderUtils, SeriesScanUtil, and DeviceCollector
  wrap IOException/RuntimeException as CorruptedTsFileException
- Normal queries: message mentions corruption, tells user to check logs (no file path)
- read_tsfile queries: message includes full TsFile path
- DiskChunkLoader/DiskAlignedChunkLoader expose getTsFile() for File reference
- ErrorHandlingUtils handles CorruptedTsFileException with TSFILE_PROCESSOR_ERROR
- AbstractDriverThread matches CorruptedTsFileException by instanceof
- AbstractDriverThread catch CorruptedTsFileException as abort cause
- i18n: add EXCEPTION_ constants for all five stages (en + zh)
- IT: IoTDBQueryWithCorruptedTsFileIT tests metadata index and page data corruption
CorruptedTsFileException is a RuntimeException and the DataNode handles it
fine without wrapping, so the catch-and-rewrap block is redundant.
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 14.06250% with 110 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.08%. Comparing base (965e863) to head (5a3f91d).
⚠️ Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
...n/tvf/read_tsfile/ExternalTsFileQueryResource.java 0.00% 42 Missing ⚠️
...ine/execution/operator/source/FileLoaderUtils.java 15.78% 32 Missing ⚠️
...e/iotdb/db/exception/CorruptedTsFileException.java 0.00% 14 Missing ⚠️
...ngine/execution/schedule/AbstractDriverThread.java 14.28% 12 Missing ⚠️
...gine/execution/operator/source/SeriesScanUtil.java 46.66% 8 Missing ⚠️
.../org/apache/iotdb/db/utils/ErrorHandlingUtils.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18304      +/-   ##
============================================
+ Coverage     43.02%   43.08%   +0.05%     
  Complexity      374      374              
============================================
  Files          5362     5363       +1     
  Lines        381599   381773     +174     
  Branches      49553    49586      +33     
============================================
+ Hits         164192   164474     +282     
+ Misses       217407   217299     -108     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

* TsFileRuntimeException} so it follows the existing TsFile error handling and bypasses all {@code
* catch (IOException)} blocks in the operator hierarchy.
*/
public class CorruptedTsFileException extends TsFileRuntimeException {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extends IoTDBRuntimeException, add a new TSStatusCode for it, then add a else-if branch in ErrorHandlingUtils.onQueryException which won't print exception stack and only print message to info log

// itself and we can match it here.
CorruptedTsFileException corruptedTsFileException =
(CorruptedTsFileException) rootCause;
logger.warn(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's external tsfile, info is enough, for internal tsfile, maybe warn is better.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@shuwenwei
shuwenwei requested a review from JackieTien97 July 27, 2026 04:19

@JackieTien97 JackieTien97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional findings after reviewing the latest revision.

}

return timeSeriesMetadata;
} catch (Exception e) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Please preserve non-TsFile failures before wrapping. This try also covers context.getPathModifications(...); its loader can throw MemoryNotEnoughException while reserving matched-mod memory. For a closed, healthy file, this catch converts QUERY_EXECUTION_MEMORY_NOT_ENOUGH into a corruption error and loses the original status. Narrow the catch to reader/deserialization operations or rethrow typed IoTDBRuntimeExceptions first. The aligned path below has the same issue.

}
return chunkReader.loadPageReaderList();
try {
return chunkReader.loadPageReaderList();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] For current disk readers this catch is unreachable: ChunkReader and AbstractAlignedChunkReader build page readers in constructors called by getChunkReader() above, while AbstractChunkReader.loadPageReaderList() only returns the prebuilt list. Page-header construction (and aligned time-page decompression) is therefore reported as READ_CHUNK_DATA, never LOAD_PAGE_READER. Please move the stage boundary to where construction occurs or remove/collapse this stage, and add an exact-stage test.


// Corrupt the nodeType byte to 0xFF (all valid types are 0-3)
byte[] fileBytes = Files.readAllBytes(tsFile.toPath());
fileBytes[(int) fileMetadataPos - 1] = (byte) 0xFF;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] fileMetadataPos - 1 does not locate a device-index node in this fixture. With two devices, the device root is stored in TsFileMetadata; this byte belongs to the last measurement-index node. The passing IT consequently reports READ_TIMESERIES_METADATA, matching the broad assertion below, and never exercises READ_METADATA_INDEX_NODE/DeviceCollector. Force an on-disk device-index node or locate its offset explicitly, then assert the exact stage/message.

int dataStart = magicLen + Byte.BYTES;
int dataEnd = (int) metaOffset;
// Corrupt bytes in the middle of the data section — XOR 512 bytes to ensure decompression fails
int middle = dataStart + (dataEnd - dataStart) / 2;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] The midpoint is not a page-data boundary. In the current run both this and the normal-query case corrupt a chunk header and report READ_CHUNK_DATA, so DECODE_PAGE_DATA is untested and layout/compressor changes can alter the failure stage. Please locate a concrete page payload, fully consume the ResultSet, and assert the exact stage-specific message/log.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants